feat: implement should_apply_proposer_boost for gloas - #9233
Merged
Conversation
Implement the `should_apply_proposer_boost` logic from consensus-specs commit 71d1151 (PR #4807). This addresses the builder reveal safety concern where a colluding next-slot proposer could use proposer boost to override a legitimately revealed block. Changes: - Add `ptcTimeliness` and `proposerIndex` fields to ProtoBlock - Add `isBlockPtcTimely` to track PTC deadline timeliness - Add `shouldApplyProposerBoost` which withholds boost when the parent is a weak, equivocating block from the previous slot - Add `findEquivocatingBlocks` in ProtoArray to detect proposer equivocations by scanning for PTC-timely blocks at the same slot from the same proposer - Gate proposer boost in `getWeight` on `shouldApplyProposerBoost()` - Pre-gloas blocks retain unconditional boost (backward compatible) Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Code Review
This pull request implements the Gloas fork-choice logic for proposer boost and PTC (Payload Timeliness Committee) timeliness. It adds ptcTimeliness and proposerIndex to block metadata and introduces the shouldApplyProposerBoost logic, which considers parent block weight and proposer equivocations. Feedback focuses on preventing a crash during the fork transition by using dynamic payload status for parent nodes and optimizing the performance of the equivocation check to avoid O(N) map iterations during head updates.
ensi321
marked this pull request as draft
April 18, 2026 02:25
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 9bc6eb6ce5
ℹ️ About Codex in GitHub
Codex has been enabled to automatically review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".
Contributor
Performance Report✔️ no performance regression detected Full benchmark results
|
4 tasks
GrapeBaBa
added a commit
to GrapeBaBa/lodestar
that referenced
this pull request
May 1, 2026
Wire the consensus-specs Fork Choice Compliance suite (ChainSafe#3831) into the existing `forkChoiceTest` runner. The on-disk layout matches the standard spec-test layout (`tests/<preset>/<fork>/fork_choice_compliance/<handler>/<suite>/<case>/`), so it slots in alongside `fork_choice` and `sync` runners. Three test-only accommodations the compliance fixtures require: 1. `bls_setting: 2` — every compliance fixture uses placeholder signatures. Pass `validSignatures: testcase.meta?.bls_setting !== BigInt(1)` to `chain.processBlock` so verification short-circuits. Standard `fork_choice` fixtures use `bls_setting: 1` so behavior there is unchanged. 2. `BLOCK_ERROR_ALREADY_KNOWN` — compliance fixtures intentionally re-import the same block (`dup_shift` mutations in their `meta.yaml`). Spec semantics for `on_block(store, known_block)` is a no-op success. Production block import correctly rejects with ALREADY_KNOWN; this runner treats that case as success only when the step is `valid: true`. 3. Cross-epoch attestation shuffling — `on_attestation` decodes aggregation_bits using the state at the attestation's target checkpoint, not the head state. The runner now resolves the right shuffling via ShufflingCache + regen (mirroring the production validation path) instead of `headState.epochCtx.getIndexedAttestation`, which only worked when the attestation's epoch happened to be in the head's epoch cache (±1 epoch) and broke on cross-epoch fork attestations surfaced by the compliance suite. Adds support for two compliance-only check fields: - `viable_for_head_roots_and_weights` (consensus-specs#3831): compared via `getViableHeads()`. Both sides are sorted by root before comparison since the spec doesn't fix order. - `head_payload_status` (gloas): mapped between our internal enum ordering (PENDING=0, EMPTY=1, FULL=2) and spec ordering (EMPTY=0, FULL=1, PENDING=2). Pass rate against the latest comptests workflow `small.tar.gz` artifact: fulu/fork_choice_compliance: 253/1472 cases pass (17.2%) Top remaining failures: - ~80% `Invalid proposer boost root` — consensus-specs#4807 introduced a `block.proposer_index == get_beacon_proposer_index(head_state)` guard in `update_proposer_boost_root` that we do not yet implement; affects all forks (not just gloas equivocation handling). Tracked for follow-up alongside ChainSafe#9233. - ~1% `Invalid viable heads` — proposer-boost rounding on minimal preset (see `getViableHeads()` weight note).
GrapeBaBa
added a commit
to GrapeBaBa/lodestar
that referenced
this pull request
May 1, 2026
Wire the consensus-specs Fork Choice Compliance suite (ChainSafe#3831) into the existing `forkChoiceTest` runner. The on-disk layout matches the standard spec-test layout (`tests/<preset>/<fork>/fork_choice_compliance/<handler>/<suite>/<case>/`), so it slots in alongside `fork_choice` and `sync` runners. Three test-only accommodations the compliance fixtures require: 1. `bls_setting: 2` — every compliance fixture uses placeholder signatures. Pass `validSignatures: testcase.meta?.bls_setting !== BigInt(1)` to `chain.processBlock` so verification short-circuits. Standard `fork_choice` fixtures use `bls_setting: 1` so behavior there is unchanged. 2. `BLOCK_ERROR_ALREADY_KNOWN` — compliance fixtures intentionally re-import the same block (`dup_shift` mutations in their `meta.yaml`). Spec semantics for `on_block(store, known_block)` is a no-op success. Production block import correctly rejects with ALREADY_KNOWN; this runner treats that case as success only when the step is `valid: true`. 3. Cross-epoch attestation shuffling — `on_attestation` decodes aggregation_bits using the state at the attestation's target checkpoint, not the head state. The runner now resolves the right shuffling via ShufflingCache + regen (mirroring the production validation path) instead of `headState.epochCtx.getIndexedAttestation`, which only worked when the attestation's epoch happened to be in the head's epoch cache (±1 epoch) and broke on cross-epoch fork attestations surfaced by the compliance suite. Adds support for two compliance-only check fields: - `viable_for_head_roots_and_weights` (consensus-specs#3831): compared via `getViableHeads()`. Both sides are sorted by root before comparison since the spec doesn't fix order. - `head_payload_status` (gloas): mapped between our internal enum ordering (PENDING=0, EMPTY=1, FULL=2) and spec ordering (EMPTY=0, FULL=1, PENDING=2). Pass rate against the latest comptests workflow `small.tar.gz` artifact: fulu/fork_choice_compliance: 253/1472 cases pass (17.2%) Top remaining failures: - ~80% `Invalid proposer boost root` — consensus-specs#4807 introduced a `block.proposer_index == get_beacon_proposer_index(head_state)` guard in `update_proposer_boost_root` that we do not yet implement; affects all forks (not just gloas equivocation handling). Tracked for follow-up alongside ChainSafe#9233. - ~1% `Invalid viable heads` — proposer-boost rounding on minimal preset (see `getViableHeads()` weight note).
GrapeBaBa
added a commit
to GrapeBaBa/lodestar
that referenced
this pull request
May 1, 2026
Wire the consensus-specs Fork Choice Compliance suite (ChainSafe#3831) into the existing `forkChoiceTest` runner. The on-disk layout matches the standard spec-test layout (`tests/<preset>/<fork>/fork_choice_compliance/<handler>/<suite>/<case>/`), so it slots in alongside `fork_choice` and `sync` runners. Three test-only accommodations the compliance fixtures require: 1. `bls_setting: 2` — every compliance fixture uses placeholder signatures. Pass `validSignatures: testcase.meta?.bls_setting !== BigInt(1)` to `chain.processBlock` so verification short-circuits. Standard `fork_choice` fixtures use `bls_setting: 1` so behavior there is unchanged. 2. `BLOCK_ERROR_ALREADY_KNOWN` — compliance fixtures intentionally re-import the same block (`dup_shift` mutations in their `meta.yaml`). Spec semantics for `on_block(store, known_block)` is a no-op success. Production block import correctly rejects with ALREADY_KNOWN; this runner treats that case as success only when the step is `valid: true`. 3. Cross-epoch attestation shuffling — `on_attestation` decodes aggregation_bits using the state at the attestation's target checkpoint, not the head state. The runner now resolves the right shuffling via ShufflingCache + regen (mirroring the production validation path) instead of `headState.epochCtx.getIndexedAttestation`, which only worked when the attestation's epoch happened to be in the head's epoch cache (±1 epoch) and broke on cross-epoch fork attestations surfaced by the compliance suite. Adds support for two compliance-only check fields: - `viable_for_head_roots_and_weights` (consensus-specs#3831): compared via `getViableHeads()`. Both sides are sorted by root before comparison since the spec doesn't fix order. - `head_payload_status` (gloas): mapped between our internal enum ordering (PENDING=0, EMPTY=1, FULL=2) and spec ordering (EMPTY=0, FULL=1, PENDING=2). Pass rate against the latest comptests workflow `small.tar.gz` artifact: fulu/fork_choice_compliance: 253/1472 cases pass (17.2%) Top remaining failures: - ~80% `Invalid proposer boost root` — consensus-specs#4807 introduced a `block.proposer_index == get_beacon_proposer_index(head_state)` guard in `update_proposer_boost_root` that we do not yet implement; affects all forks (not just gloas equivocation handling). Tracked for follow-up alongside ChainSafe#9233. - ~1% `Invalid viable heads` — proposer-boost rounding on minimal preset (see `getViableHeads()` weight note).
GrapeBaBa
added a commit
to GrapeBaBa/lodestar
that referenced
this pull request
Jul 7, 2026
Wire the consensus-specs Fork Choice Compliance suite (ChainSafe#3831) into the existing `forkChoiceTest` runner. The on-disk layout matches the standard spec-test layout (`tests/<preset>/<fork>/fork_choice_compliance/<handler>/<suite>/<case>/`), so it slots in alongside `fork_choice` and `sync` runners. Three test-only accommodations the compliance fixtures require: 1. `bls_setting: 2` — every compliance fixture uses placeholder signatures. Pass `validSignatures: testcase.meta?.bls_setting !== BigInt(1)` to `chain.processBlock` so verification short-circuits. Standard `fork_choice` fixtures use `bls_setting: 1` so behavior there is unchanged. 2. `BLOCK_ERROR_ALREADY_KNOWN` — compliance fixtures intentionally re-import the same block (`dup_shift` mutations in their `meta.yaml`). Spec semantics for `on_block(store, known_block)` is a no-op success. Production block import correctly rejects with ALREADY_KNOWN; this runner treats that case as success only when the step is `valid: true`. 3. Cross-epoch attestation shuffling — `on_attestation` decodes aggregation_bits using the state at the attestation's target checkpoint, not the head state. The runner now resolves the right shuffling via ShufflingCache + regen (mirroring the production validation path) instead of `headState.epochCtx.getIndexedAttestation`, which only worked when the attestation's epoch happened to be in the head's epoch cache (±1 epoch) and broke on cross-epoch fork attestations surfaced by the compliance suite. Adds support for two compliance-only check fields: - `viable_for_head_roots_and_weights` (consensus-specs#3831): compared via `getViableHeads()`. Both sides are sorted by root before comparison since the spec doesn't fix order. - `head_payload_status` (gloas): mapped between our internal enum ordering (PENDING=0, EMPTY=1, FULL=2) and spec ordering (EMPTY=0, FULL=1, PENDING=2). Pass rate against the latest comptests workflow `small.tar.gz` artifact: fulu/fork_choice_compliance: 253/1472 cases pass (17.2%) Top remaining failures: - ~80% `Invalid proposer boost root` — consensus-specs#4807 introduced a `block.proposer_index == get_beacon_proposer_index(head_state)` guard in `update_proposer_boost_root` that we do not yet implement; affects all forks (not just gloas equivocation handling). Tracked for follow-up alongside ChainSafe#9233. - ~1% `Invalid viable heads` — proposer-boost rounding on minimal preset (see `getViableHeads()` weight note).
Member
|
@ensi321 is this still relevant? |
Hardcoding PayloadStatus.PENDING in the weak-parent lookup crashes at the gloas fork transition: the first gloas block's parent is pre-gloas (FULL-only), and getNodeIndexByRootAndStatus throws INVALID_NODE_INDEX when PENDING is requested for a pre-gloas root. The adjacent-parent guard does not short-circuit this path at the boundary. Use parentBlock.payloadStatus instead, matching the pattern already used for the strong-variant lookup elsewhere in this file. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Member
Author
For some reason, our unstable is now passing these tests even without this PR and we failed them previously lodestar/packages/beacon-node/test/spec/presets/fork_choice.test.ts Lines 738 to 742 in e6d38ec But we still need to implement Added some new tests ethereum/consensus-specs#5441 to make sure this is covered. |
ensi321
marked this pull request as ready for review
July 9, 2026 14:36
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## unstable #9233 +/- ##
=========================================
Coverage 52.49% 52.49%
=========================================
Files 848 848
Lines 60487 60486 -1
Branches 4466 4465 -1
=========================================
Hits 31755 31755
Misses 28670 28670
+ Partials 62 61 -1 🚀 New features to boost your workflow:
|
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: d2bb9684f2
ℹ️ About Codex in GitHub
Codex has been enabled to automatically review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".
twoeths
reviewed
Jul 14, 2026
|
|
||
| // Indicate whether block arrives in a timely manner ie. before the 4 second mark | ||
| // Spec: Store.block_timeliness[ATTESTATION_TIMELINESS_INDEX] | ||
| timeliness: boolean; |
Member
There was a problem hiding this comment.
we may want to use a single block_timeliness: number to represent these 2 flags, cc @wemeetagain to confirm
Member
Author
There was a problem hiding this comment.
Is there a real benefit to this? Using bits impact readability, and we are only saving one boolean field per ProtoNode
This was referenced Jul 16, 2026
wemeetagain
pushed a commit
that referenced
this pull request
Jul 24, 2026
**Motivation** - is_head_weak() is needed for should_apply_proposer_boost() #9233 - we also need it for `getProposerHead()` **Description** - implement cross-fork [is_head_weak()](https://github.com/ethereum/consensus-specs/blob/6408b990c0b4e58d4f50ae49b59384e82e5e06d8/specs/gloas/fork-choice.md?plain=1#L739) spec api and use it in `getProposerHead()` - track attestation weight in ProtoNode separately, weight still means total weight - back propagate boost deltas and attestation deltas separately - also implement is_parent_strong() **AI Assistance Disclosure** - created with the help of Claude --------- Co-authored-by: Tuyen Nguyen <twoeths@users.noreply.github.com> Co-authored-by: bing <spiralladder@fastmail.com>
…roposer-boost # Conflicts: # packages/beacon-node/test/spec/presets/fork_choice.test.ts
…ProposerBoost is_head_weak() landed via #9654 with the exact gloas semantics (boost-excluded attestation score + committee-scoped equivocator balance add-back), so the weak-parent check can call it directly instead of approximating with raw node.weight against the reorg threshold. Also adapt findEquivocatingBlocks to the current VariantIndices layout via getDefaultNodeIndex, and add the new ProtoBlock fields to test fixtures that were added on unstable since the last merge. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
should_apply_proposer_boost judges the parent via is_head_weak against the attestations known to the store, but the decision ran before applyScoreChanges() landed this round's deltas, so it read the previous round's attestation scores. A parent crossing the weak threshold on the pending batch kept the boost withheld, and a newly detected equivocator kept its discounted vote counted. For a gloas boosted block, split the score update in two passes: attestation deltas first, then the boost decision, then the boost deltas. Pre-gloas keeps the single pass since the boost is unconditional there. Also replace findEquivocatingBlocks() with hasEquivocatingBlock(): the only caller tests emptiness, so return on first match instead of allocating a list. The two ordering tests fail on the previous commit and pass on this one; the spec vectors cannot catch this because the check step recomputes the head, self-healing the transient decision before assertions. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Measures the two-pass applyScoreChanges overhead against the single-pass pre-gloas row: 2.96 ms/op vs 2.72 ms/op at vc 600k bc 64 (~9%), paid only while a gloas block holds proposer boost. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
nflaig
reviewed
Aug 4, 2026
nflaig
reviewed
Aug 4, 2026
nflaig
reviewed
Aug 4, 2026
Comment on lines
+593
to
+595
| this.protoArray.applyScoreChanges({attestationDeltas, proposerBoost: null, ...checkpoints}); | ||
| const proposerBoost = this.shouldApplyProposerBoost() ? this.getProposerBoost() : null; | ||
| this.protoArray.applyScoreChanges({ |
Member
There was a problem hiding this comment.
is calling applyScoreChanges twice fine? cc @twoeths
Member
There was a problem hiding this comment.
right now applyScoreChanges() does 2 things:
- (1) update node's weights
- (2) update best child/descendants
we can improve by calling 1) twice and call 2) in the end
it's more like a refactor and does not affect fulu so we can just leave a TODO and address in another PR I guess
twoeths
reviewed
Aug 4, 2026
Member
Author
|
Going to reflect the changes from ethereum/consensus-specs#5515 also |
ensi321
added a commit
that referenced
this pull request
Aug 26, 2026
The alpha.14 comptests were regenerated, so the seed-embedding case names changed again. Full re-run and re-triage of the gloas suite: - #9821 (exact Gwei weights) fixed the proposer boost weight quantization divergence (#9694); that skip bucket is removed. - #9864 (recompute head after pulling up checkpoints) fixed the stale cached head read in the proposer boost dependent-root gate (#9666); both remaining pins are removed. - 127 cases remain skipped for consensus-specs#5496 (childless payload-status variants bypass the filter_block_tree FFG check), regenerated names. This is the only remaining failure bucket. Suite result with #9233 merged locally: 1345 passed, 0 failed. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_018Vw1BUX36pwcv52fGr3n1C
nflaig
added a commit
that referenced
this pull request
Aug 31, 2026
Implement the proposer equivocation branch of `get_proposer_head` from ethereum/consensus-specs#4807, which reorgs a weak previous-slot head whose proposer is known to have equivocated: ```python elif all([head_weak, current_time_ok, proposer_equivocation]): return parent_node ``` This is the proposer side of the builder reveal safety argument that #9233 implemented the attester side of. A builder reveals once it sees a block with enough weight and no equivocation. If the proposer then publishes an equivocating block and the next proposer extends it, attesters withhold the boost via `should_apply_proposer_boost`, but for the equivocating block to actually be reorged the next proposer has to build on the parent, which is this branch. It lives in the phase0 spec so it applies pre-gloas as well. - `getProposerHead` returns the parent when the head is weak, from the previous slot and another block at the same slot from the same proposer is in fork choice, skipping the regular reorg conditions (`head_late`, `ffg_competitive`, `finalization_ok`, `proposing_on_time`, `parent_strong`). The boost worn off check stays a precondition for both branches, the spec asserts it before either - `shouldOverrideForkChoiceUpdate` mirrors the branch so the fcu skip at import and the payload attributes prediction in `prepareNextSlot` follow `getProposerHead`. It does not check `is_head_weak`, the head slot's attestations are still queued when it runs so the head is assumed weak, same as the regular branch - `hasEquivocatingBlock` takes a `ptcTimelyOnly` flag, `should_apply_proposer_boost` only counts PTC-timely siblings while `is_proposer_equivocation` counts any known block - the gloas parent payload status is preserved, `parentBlock` is already resolved via `getParentPayloadStatus` - link the `is_proposer_equivocation`, `should_apply_proposer_boost` and `record_block_timeliness` specrefs to their implementations and bump the `get_proposer_head` spec link, `v1.4.0-beta.4` predates this branch The existing check order and `notReorgedReason` values are unchanged, the new branch only runs ahead of them. Equivocating blocks reach fork choice since #9805, before that this could only trigger for blocks fetched via unknown block sync. There are no spec vectors for this branch upstream, coverage is via the `getProposerHead` and `shouldOverrideForkChoiceUpdate` unit tables. The four reorg cases fail on `unstable` and pass here, and the `get_proposer_head`, `reorg`, `should_override`, `ex_ante` and `should_apply_proposer_boost` fork choice spec tests still pass. Closes #9764
This was referenced Sep 1, 2026
Member
|
🎉 This PR is included in v1.47.0 🎉 |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
6 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Implements the
should_apply_proposer_boostlogic ethereum/consensus-specs/pull/4807 (gloas/fork-choice.md#new-should_apply_proposer_boost).Was blocked on
is_head_weak(); now rebased on top of #9654 (merged) and reuses itsisHeadWeak()— boost-excluded attestation score + equivocator balance add-back — for the weak-parent check, exactly matching the spec'sis_head_weak(store, parent_root)call.Changes
ptcTimelinessandproposerIndexfields toProtoBlockisBlockPtcTimelyto track PTC deadline timelinessshouldApplyProposerBoostwhich withholds boost when the parent is a weak, equivocating block from the previous slotisHeadWeak()from feat: implement forkchoice is_head_weak() and is_parent_strong() #9654 for the weak-parent checkfindEquivocatingBlocksinProtoArrayto detect proposer equivocationsgetWeightonshouldApplyProposerBoost()Note on test coverage
The existing gloas fork_choice vectors pass with or without this gate (the withhold branch executes but never decides the asserted head), so CI alone doesn't discriminate it. Discriminating vectors are proposed upstream in consensus-specs (
test_should_apply_proposer_boost.py, MC/DC over the three apply conditions); the withheld case fails onunstableand passes on this branch.AI Assistance Disclosure
Created with the help of Claude.
🤖 Generated with Claude Code